Global error handler cleanup - Metrics View #2249
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2249 +/- ##
=====================================
Coverage 79.5% 79.6%
=====================================
Files 121 121
Lines 20950 20938 -12
=====================================
- Hits 16672 16668 -4
+ Misses 4278 4270 -8 ☔ View full report in Codecov by Sentry. |
| global::handle_error(MetricsError::Config(format!( | ||
| "no criteria provided, dropping view. mask: {mask:?}" | ||
| ))); | ||
| otel_warn!( |
There was a problem hiding this comment.
for a method that returns Result, I don't think there is any need of internal logging. If we really want to do some log, then it should be Debug level.
There was a problem hiding this comment.
The actual error is getting lost in all these cases, as we just return success with empty view.
There was a problem hiding this comment.
if we always return success, then that looks like a flaw. Since it is not introduced in this PR, lets remove the error logging from here, and put a todo (optional). We need to rework the View and can fix these then.
There was a problem hiding this comment.
Added TODO, and removed logging.
Fixes #
Design discussion issue (if applicable) #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes